Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve simgetimages speed #1910

Merged
merged 2 commits into from
Apr 30, 2019
Merged

Conversation

madratman
Copy link
Contributor

@madratman madratman commented Apr 24, 2019

(Don't merge until we find the right format for depth)

This PR improves speed of simgetimages by :

Let's talk about RGB first:

  • Behind the scenes, Unreal initializes the render target to RGBA16f as can be seen in the engine code here

  • The suggestion in Improve image capture speed #1818 works well in that we can initialize it to EPixelFormat::PF_B8G8R8A8.
    There's also an EPixelFormat::PF_R8G8B8A8_UINT, but trying that makes the engine crash with assert failure in that the requested render target format is not supported.
    Interestingly, PF_R8G8B8A8 leads to the same assert failure.
    There's no PF_B8G8R8A8_UINT, unfortunately.
    Side note: these names are misleading in that BGR is actually RGB - https://answers.unrealengine.com/questions/69615/are-epixelformats-pf-b8g8r8a8-and-pf-r8g8b8a8-flip.html
    Why engine crashes though, is something I am not sure about.

  • Q. We should try to get int8 images / textures? We can hopefully change the scene capture to get rgba int (not float) and then use PF_R8G8B8A8_UINT? Although I am not sure if this will help coz the GPU would have the images as floats only.

Moving on to depth:

  • Now, there should be a similar optimization possible for render targets for depth images. As far as I understand, it should be EPixelFormat::PF_R16F as alluded by this line https://github.com/Microsoft/AirSim/blob/master/Unreal/Plugins/AirSim/Source/RenderRequest.cpp#L127. However doing so, again leads to engine crashing with assert failure in that the requested render target format is not supported .
    Not completely sure why is that++
    My hunch is this happens coz the capture source is being initialized to ESceneCaptureSource::SCS_FinalColorLDR for all image types at this line https://github.com/Microsoft/AirSim/blob/master/Unreal/Plugins/AirSim/Source/PIPCamera.cpp#L66.

  • ESceneCaptureSource::SCS_FinalColorLDR doesn't seem efficient for depth images. I tried changing it to ESceneCaptureSource::SCS_SceneDepth, similarly there's a ESceneCaptureSource::SCS_Normal which should be used for the normal image?
    Unfortunately, engine crashes yet again with those two options.

  • Anyhow, for now, I have added a placeholder TMap for image_types to pixel_format types. The map is only used for all non depth image types (aka image_type==0 || image_type==5 || image_type==6 || image_type==7).
    for depth images, I still call initautoformat for now.

P.S. Technically, we should use ETextureRenderTargetFormat, which is the set of EPixelFormats exposed to rendertarget.
However, the mapping is fairly intuitive from the variable names as can be seen in the engine code in these lines, so, i am sticking with pixelformat

@madratman madratman requested a review from sytelus April 25, 2019 00:48
@madratman
Copy link
Contributor Author

update:
This call to FTextureRenderTargetResource::IsSupportedFormat() was the assertion failure,

FTextureRenderTargetResource::IsSupportedFormat() can be seen in these lines, which means only these pixel formats are supported:

PF_B8G8R8A8:
PF_A16B16G16R16:
PF_FloatRGB:
PF_FloatRGBA: // for exporting materials to .obj/.mtl
PF_A2B10G10R10: //Pixel inspector for normal buffer
PF_DepthStencil: //Pixel inspector for depth and stencil buffer

@madratman madratman force-pushed the PR/simgetimages branch 3 times, most recently from 07cc26c to d83c7c4 Compare April 25, 2019 20:26
@madratman
Copy link
Contributor Author

update:
i tried PF_DepthStencil with ESceneCaptureSource::SCS_SceneDepth in vain. With that we have a weird memory writing error in the unreal stack. I guess as part of this in a shader function, which I haven't looked a lot into.

Interestingly, ESceneCaptureSource::SCS_SceneDepth works fine with PF_B8G8R8A8, but the scaling of the depth image blows up a lot, so not messing with it anymore.

Now, this PR is good to be merged from my end. For depth images (image_type = 2/3/4), we'll have placeholder formats in APIPCamera::image_type_to_pixel_format_map_, but they are not being used as can be seen in APIPCamera::setupCameraFromSettings()

fwiw, I found a similar discussion in carla here carla-simulator/carla#750 with unfortunately similar conclusions

…ormat to RGBA8 in PIPCamera.cpp, and only copying RGB in RenderRequest.cpp
remove old code's np.flipuds
replace airsim.write_png with cv2.imwrite
@madratman madratman merged commit 0d7cf52 into microsoft:master Apr 30, 2019
@madratman madratman mentioned this pull request Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant